gguf: release the weight mmap and the split loader's overflow chunks - #347

core_gguf::load_weights maps the whole GGUF and hands the region to the device on the zero-copy
path. buffer_from_host_ptr has no deallocator parameter, and Metal passes deallocator:nil, so
freeing the backend buffer released the device-side view and left the host mapping in place. The
loader dropped its only handle to a region it still owned — lookup_gpu_mmap, the function that
would find it again, had no caller on any release path.

The result on Apple silicon is that a process holds every weight file it has ever loaded. Two Omni
CTC sessions in one process (3B q8 then 7B q8, serial) peaked at 11.25 GB, the sum of both
models, against 7.73 GB with CRISPASR_GGUF_MMAP=0. The accumulation is per load, not per
file: twenty load/free cycles of a single GGUF left twenty live mappings.

The comment being deleted is wrong, and here is the measurement

gguf_loader.cpp argued the leak was affordable because "on macOS the kernel can still evict
file-backed pages under pressure (they're not anonymous)". That is false for this mapping.
MappedFile(path, /*writable=*/true) maps MAP_PRIVATE with PROT_READ|PROT_WRITE — the
writable flag exists for backends that fold weights in place after load, e.g. parakeet's
batch-norm-into-conv — and macOS resolves the private copy as each page faults in. So merely
reading the weights privatizes them.

A standalone probe that reads one byte per page and writes nothing anywhere in the program takes a
658 MB GGUF to:

mapped file   [ 658.7M  658.7M  658.7M ]  rw-/rwx SM=COW   <model>.gguf
                VIRT    RESID    DIRTY

The control is the same file mapped MAP_SHARED | PROT_READ and handed to
ggml_backend_dev_buffer_from_host_ptr:

mapped file   [ 658.7M  658.7M      0K ]  r--/r-x SM=SHM   <model>.gguf

— fully resident, zero dirty, and still zero after the backend buffer is freed. Dirty private
pages can be compressed or swapped, never dropped, so the cost is real memory for the life of the
process. The mapping mode is the whole cause. A read-only consumer that never folds weights could
take the MAP_SHARED|PROT_READ path instead and would not need any of this; that option is worth
knowing about but it is not what the loader does today.

The CPU mmap path was never affected — its mmap_buffer_free calls munmap. This is
Apple-silicon-specific, or more precisely specific to any device advertising
buffer_from_host_ptr.

The fix

core_gguf::release_weight_buffer(ggml_backend_buffer_t&): take-and-erase the side-map entry in one
critical section, free the backend buffer, then unmap, then null the caller's handle.

• Take before free, not after. Between a free and a later erase, a concurrent load could receive
  a new buffer at the same address and register it, and the erase would drop a live mapping's record.
• Unmap after the free, so a device-side view never outlives its pages. This inherits ggml's
  existing caller contract rather than adding one — ggml_metal_buffer_free already vm_deallocates
  the host pages of a buffer it owns, so "no work referencing this buffer may still be in flight" is
  a precondition every caller already meets.
• A missing entry is the normal case, not an error: the CPU mmap path and the legacy alloc+copy
  path register nothing and reach the same call.

No vtable entry is touched. An earlier attempt replaced iface.free_buffer on the inner buffer,
reasoning that context and every other method stay Metal's own. That is wrong: that pointer is
Metal's type tag. ggml_backend_buffer_is_metal classifies a buffer by comparing iface.free_buffer
against the shared and private Metal free callbacks (ggml-metal.cpp:176-179), and these buffers are
built with ggml_backend_metal_buffer_shared_i, so a substitution would unclassify every weight
buffer. This fork uses the same pointer as a type tag itself, in mmap_advise_random.

If you would prefer a universal hook, the way to make one safe is to classify a Metal buffer by
its buft — the idiom ggml_backend_metal_device_supports_buft already uses — after which a vtable
shim releases the mapping for every model at once and needs no per-backend change. That is a
ggml-metal change rather than a CrispASR one, so it is offered here rather than included.

A second leak on the same page

load_weights_split allocates a partition above the 1.5 GiB chunk limit as several backend buffers,
putting the first in WeightLoad::buf/::buf_cpu and the rest in ::split_bufs, documented as the
caller's to free. All eighteen backends that call it drop the vector — each moves buf and
buf_cpu into its model struct and lets the local WeightLoad die. free_weights() frees them
correctly and is reached by none of those eighteen. The chunk limit is unconditional, so this is not
only the AMD Vulkan case it was written for.

Ownership moved into the loader: the overflow chunks are keyed to the first buffer of their own
partition and released with it, which fixes all eighteen without touching any of them.
split_bufs remains as a read-only listing of how a load was partitioned. Happy to invert this if
you would rather keep caller ownership and patch the eighteen call sites instead — but a contract
that every caller has ignored seemed like the wrong place for the obligation.

Third leak: a rejected GGUF

The GPU bounds-check rejection set out.buf = nullptr and fell through to the legacy loader,
abandoning the backend buffer and the whole-file mapping registered against it — neither freed nor
unmapped nor erased. That branch is reached by a truncated or crafted GGUF, so it was reachable on
demand by untrusted input. It now releases.

gguf_loader.cpp contains exactly one ggml_backend_buffer_free call, inside
release_weight_buffer.

Call-site conversion

136 sites across 90 files. The set was derived by provenance, not by name: a free is converted only
where its handle was assigned from a WeightLoad field, and never where the same expression is also
assigned from ggml_backend_alloc_ctx_tensors / alloc_buffer / buft_alloc_buffer. That second
rule keeps kv_buf, buf_perm, cross_kv_buf, fused_buf, bake_buf and buf_f32 out. Every
case where the two rules disagreed was checked by hand — titanet's g.buf, moss_tts_codec's and
moss_tts_local_codec's w_buf, and indextts's beam-pool buf are separately allocated and stay
as they were.

Five classes escape per-file analysis and were handled directly:

crisp_audio is the one that got away, and it is worth saying how. It is outside src/, so the
qualified-call scan never looked at it; and it has no src/ twin, so test-copies-in-sync.cpp —
which enumerates {crisp_punc, crisp_lid, crisp_truecase} and compares each file against a src/
copy — structurally could not see it either. It surfaced on a re-run of the provenance rule over
every tracked C/C++ file rather than over src/: all 1201 of them outside ggml/ and third_party/,
intersecting "assigned from a WeightLoad field" with "passed to ggml_backend_buffer_free". That
sweep returns exactly two hits — crisp_audio/src/audio_tower.cpp:688 and a trailing-name collision
in titanet (ctx->g.buf freed, ctx->buf loaded) that the by-hand list above already cleared.

crisp_audio has two in-tree consumers, qwen3_asr and higgs_stt, and one out-of-tree one,
CrispEmbed's BidirLM-Omni audio path.

Left unconverted, verified rather than assumed: the seven backends that already free through
core_gguf::free_weights; crispasr.cpp, whose Whisper loader allocates a backend-owned buffer with
ggml_backend_alloc_ctx_tensors_from_buft and never maps; and crispasr_vad_encdec.cpp,
marblenet_vad.cpp, miotts.cpp, omnivoice.cpp and core/{attention,dac_decoder,fastconformer}.h,
whose buffers come from their own allocations.

A guard worth considering, not included here

The crisp_audio miss is the second time a name- or directory-scoped scan has been the thing that
failed, and neither the compiler nor any existing test can see it: ggml_backend_buffer_free on a
loader buffer is a legal call that compiles and runs.

The check that would catch it is mechanical, and it is the same rule the conversion above was derived
from, run as a test rather than once by hand. For every tracked production source file: collect the
expressions assigned from a WeightLoad field, collect the expressions passed to
ggml_backend_buffer_free, and fail on any name in both. Two details are what make it worth having
rather than a thing that passes while seeing nothing:

• It declares its own vocabulary and fails outside it. Every appearance of <wl>.buf /
  .buf_cpu must be classified into a form the scan understands — an assignment whose left side it
  captures, or a recognised read (free_weights, release_weight_buffer, a comparison). Anything
  else fails the test rather than being skipped. That is what would have covered the
  using namespace core_gguf spelling and the sibling-translation-unit destructor, both of which a
  plain qualified-name grep misses silently.
• Its file set is enumerated, and cross-checked against the tree — the shape
  test-copies-in-sync.cpp already uses for its pair list. A new top-level directory carrying C++
  that includes core/gguf_loader.h and is not listed fails the test. That is exactly the gap
  crisp_audio/ fell through.

It would need normalising ctx.model_buf against ctx->model_buf, and the titanet trailing-name
collision above says the comparison has to be on the whole member chain, not the last component.
Left out of this PR to keep it to the leak and its fix.

Tests

Three new/extended files, each failing before the change and passing after. None needs a model file,
a download, or a GPU to build.

tests/test-region-probe.h holds the region probe. One note in case it is useful elsewhere:
proc_regionfilename alone is not usable for this. Asked about the base of an anonymous region
it answers with the file of the next region at or above that address, so an unrelated 67 MB anonymous
neighbour was counted as a mapping of the weight file. The probe reads PROC_PIDREGIONPATHINFO,
which returns the region and its path in one record. Linux uses /proc/self/maps; elsewhere the
cases report unsupported and skip.

Deliberately not proposed: a footprint- or RSS-growth test. It samples a whole-process figure, needs
a threshold, and would be the flaky case in this suite. The region check answers the same question
exactly. The split-chunk case says in-file what it cannot prove — that the memory came back — since
ggml exposes no per-backend allocation counter.

Verification

• ctest --test-dir build -L unit -E live: 1606 passed.

• clang-format 18: clean. cppcheck: clean (the one report at qwen3_tts.cpp:3032 is pre-existing
  and only appears on cppcheck ≥ 2.17; CI's 2.7 does not produce it). clang-tidy: no new findings
  beyond the file's existing readability-braces-around-statements style.

• A downstream Rust consumer's weights-present suite — Omni CTC 1B/3B/7B, Whisper, the wav2vec2/MMS
  forced aligner, pyannote diarization, enhancement — 2059 passed / 0 failed, byte-identical
  results and identical goldens before and after.

• Memory, same machine and build, tests serial, macOS phys_footprint:

  	before	CRISPASR_GGUF_MMAP=0	after
  Omni 3B q8 + 7B q8, one process	11.245 GB	7.734 GB	7.739 GB
  four forced-aligner sessions	2.901 GB	1.367 GB	1.368 GB

  Releasing the mapping costs no wall clock — a 500-test run measured 154.6 s against 157.7 s.

The CrispEmbed half

core/gguf_loader.{h,cpp} exists in both CrispASR and CrispEmbed, and the header records a run of
commits spent recovering from drift between them. tests/test-copies-in-sync.cpp guards fourteen
duplicated files but compares paths within one checkout, so it structurally cannot see that pair —
and a new public core_gguf function is exactly the kind of change that drifts.

This PR does not stand alone: merging it without the CrispEmbed patch breaks that build.
CrispEmbed compiles three shared libraries out of this repo — crisp_audio, crisp_punc and
crisp_lid — against its own copy of the loader header, and all three now call
release_weight_buffer. Checked rather than reasoned about: with the CrispEmbed patch reverted, a
CrispEmbed build stops at crisp_audio/src/audio_tower.cpp:692 and crisp_lid/src/lid_cld3.cpp:807
with no member named 'release_weight_buffer' in namespace 'core_gguf'.

The companion PR is mculbert/CrispEmbed, branch fix/gguf-release-weight-buffer. It is not a copy
of this change — CrispEmbed's loader is a different lineage. There, the no-copy path is opt-in
(load_weights(..., try_mmap=false) by default), the mapping already lives in
WeightLoad::mmap_addr/mmap_len, and free_weights already unmaps, so CrispEmbed has no live
instance of this leak: its only two try_mmap callers keep the whole WeightLoad. What that patch
does is add release_weight_buffer with the mapping keyed to the buffer instead of only to the
WeightLoad, so the name means the same thing in both repos and the shared sources are correct
whichever header they compile against. It also closes a latent hole on that side — eleven CrispEmbed
models move wl.buf into their own struct and free it directly, each of which would leak the moment
try_mmap were switched on for it.

Its own tests/test_gguf_loader_mmap.cpp gained the region check: 1 mapping while loaded, 0 after
free_weights.

Authored by Claude Opus 5.